Skip to content

test(wip): basic harness for precision loss analysis#1462

Draft
wadealexc wants to merge 16 commits intomainfrom
alex/rounding-analysis
Draft

test(wip): basic harness for precision loss analysis#1462
wadealexc wants to merge 16 commits intomainfrom
alex/rounding-analysis

Conversation

@wadealexc
Copy link
Copy Markdown
Contributor

[DO NOT MERGE]

WIP harness for precision loss analysis. Run with forge t --mt test_rounding -vvvv

@wadealexc wadealexc force-pushed the alex/rounding-analysis branch from 4a71510 to 0b6f6d8 Compare June 11, 2025 17:29
@nadir-akhtar nadir-akhtar force-pushed the alex/rounding-analysis branch from 5e3bde6 to 685665b Compare June 11, 2025 19:20
@nadir-akhtar nadir-akhtar force-pushed the alex/rounding-analysis branch from 685665b to 622e86b Compare June 11, 2025 19:35
}

// TODO: consider incremental manual fuzzing from 1 up to WAD - 1
function test_rounding_allMagsSlashed(uint16 slashes, uint64 initialWadToSlash, uint64 _initTokenBalance, uint24 r) public rand(r) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO we should choose a lane for param generation:

  1. either use rand(r) and the _randUint method to generate all parameters
  2. or allow forge to generate all parameters and if we need additional randomness, use cheats.randomUint and similar

#1 means we should do sequential runs, supplying (1, 2, 3, 4, etc)
#2 means we just let forge decide and we only control the number of runs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or I guess there's a way to do both - let forge generate parameters via #2, but run tests using forge t --fuzz-seed <1, 2, 3, ...>

That may be most convenient?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer option #2 but open to either. My impression was we eventually want to give Foundry some more direction (e.g. slash from initialMaxMag 1 to WAD - 1), but I believe we discussed that as a parallel, second route which we would do in addition to letting Foundry fire whatever inputs it feels like

vm.pauseGasMetering();
// Bound slashes to a reasonable range, with at least 1 slash.
// Note: Runs after 2500 hit OOG errors with default gas.
slashes = uint16(bound(slashes, 1, 5000));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO number of slashes doesn't really impact the "path" so much as it defines the depth you're testing to (and overall runtime). I think this should not be a random param, but something we define.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually think it's valuable to randomize within a bound! I've just found a test where 500 slashes does not show precision loss by the end of the test, but 1 slash does.

I suggest we either do one of two things:

  1. Keep # of slashes as parameterized value (easy, more simple)
  2. Measure more granular precision loss after every operation (hard, more granular)

We can measure it how we like, but I suggest we keep slashing as a parametrized value until we have more sophisticated intermediate measurements for precision loss in the file. So we can do option 1 until option 2 is ready.

deal(address(token), address(attacker), initTokenBalance);

_magnitudeManipulation(initialWadToSlash); // Manipulate operator magnitude for a given strategy.
_deposit(initialWadToSlash); // Setup operator with new opSet as well as honest stake in same strategy.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a point we may want to change - we're allocating all available magnitude before we use multiple slashes to "get funds out".

It kinda makes sense that this wouldn't result in any loss.

What if we don't allocate all available magnitude here, and instead allocate a portion of it (parameterized). We then use multiple slashes to get a portion of the funds out via redistribution, and then attempt to withdraw remaining funds via queue+completeWithdrawals?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this is captured in partialMagsSlashed? not sure.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

partialMagsSlashed somewhat does this. It allocates all magnitude but doesn't fully slash at the end, leaving some partial residual magnitude. We can def add another test case that partially allocates.

initTokenBalance = uint64(bound(_initTokenBalance, 1, type(uint64).max));
deal(address(token), address(attacker), initTokenBalance);

_magnitudeManipulation(initialWadToSlash); // Manipulate operator magnitude for a given strategy.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kinda want to refactor this a bit to more explicitly set the parameter we're trying to set here -- we're using this method to "set max magnitude", not "initialWadToSlash"

@github-actions github-actions bot deleted the alex/rounding-analysis branch September 30, 2025 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants